Skip to content

[tests] Add DotNetRunCtrlC integration test#11293

Open
jonathanpeppers wants to merge 1 commit intomainfrom
jonathanpeppers/ctrl-c-device-test
Open

[tests] Add DotNetRunCtrlC integration test#11293
jonathanpeppers wants to merge 1 commit intomainfrom
jonathanpeppers/ctrl-c-device-test

Conversation

@jonathanpeppers
Copy link
Copy Markdown
Member

@jonathanpeppers jonathanpeppers commented May 5, 2026

Adds a device integration test that verifies Ctrl+C (SIGINT) sent to dotnet run properly stops the Android app on the device/emulator.

Test flow:

  1. Build and launch an Android app via dotnet run with WaitForExit=true
  2. Wait for app logcat output confirming launch
  3. Verify the app PID exists on the device via adb shell pidof
  4. Send SIGINT via process.SendCtrlC() extension method
  5. Assert the process exits gracefully
  6. Verify Microsoft.Android.Run's Ctrl+C handler ran ("Stopping application...")
  7. Confirm the app is no longer running on the device

Also adds a SendCtrlC() extension method on Process in ProcessExtensions.cs, using libc kill(pid, SIGINT) P/Invoke (Unix/macOS only; throws PlatformNotSupportedException on Windows).

@jonathanpeppers jonathanpeppers force-pushed the jonathanpeppers/ctrl-c-device-test branch 6 times, most recently from f65d066 to 996bb8f Compare May 6, 2026 21:59
@jonathanpeppers
Copy link
Copy Markdown
Member Author

Note that this doesn't actually fix:

I believe #11264 occurs on Windows only -- I'm still investigating.

@jonathanpeppers jonathanpeppers marked this pull request as ready for review May 7, 2026 20:49
Copilot AI review requested due to automatic review settings May 7, 2026 20:49
@jonathanpeppers
Copy link
Copy Markdown
Member Author

/review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

Android PR Reviewer completed successfully!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new device integration test to validate that sending Ctrl+C/SIGINT to dotnet run (with WaitForExit=true / Microsoft.Android.Run) triggers the tool’s cancellation handler and stops the running app on the device/emulator.

Changes:

  • Add DotNetRunCtrlC device integration test that launches an app via dotnet run, sends Ctrl+C, and verifies both tool output and app termination.
  • Add Process.SendCtrlC() test utility (Unix/macOS) to send SIGINT to a process and its descendants.
  • Adjust Microsoft.Android.Run to return exit code 130 on OperationCanceledException (Ctrl+C-style cancellation), plus a small doc comment update in DotNetCLI.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs Adds the new DotNetRunCtrlC device integration test and log capture/attachment.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Utilities/ProcessExtensions.cs Introduces SendCtrlC() helper to deliver SIGINT to dotnet run + descendants on Unix/macOS.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/DotNetCLI.cs Doc comment update for ExecuteProcess optional working directory parameter.
src/Microsoft.Android.Run/Program.cs Returns 130 when cancellation bubbles as OperationCanceledException.

Comment thread tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
Comment thread tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs Outdated
Comment thread src/Microsoft.Android.Run/Program.cs
Comment thread tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — Clean, well-structured test addition

Summary: This PR adds a device integration test (DotNetRunCtrlC) that verifies Ctrl+C (SIGINT) properly stops an Android app launched via dotnet run, along with a SendCtrlC() extension method using libc kill() P/Invoke, and a small OperationCanceledException handler in Microsoft.Android.Run/Program.cs.

What looks good

  • The OperationCanceledException → exit code 130 in Program.cs is correct (standard Unix convention) and properly ordered before the catch-all Exception handler.
  • SendCtrlC() is well-documented, references the dotnet/sdk pattern, handles the full process tree via pgrep recursion, and has an appropriate PlatformNotSupportedException guard for Windows.
  • The test follows the established DotNetRunWaitForExit pattern closely — same project setup, output capture via async handlers, try/finally cleanup with process.Kill(entireProcessTree: true), log file attachment.
  • Good layered assertions: verify app is running → send signal → verify exit → verify "Stopping application..." → verify app stopped on device.

Issues found

Severity Count
💡 Suggestion 2

CI Status

CI is still in progress (Xamarin.Android-PR internal pipeline is pending). No failures detected so far — dotnet-android and license/cla checks passed.

Generated by Android PR Reviewer for issue #11293 · ● 1.9M

@jonathanpeppers jonathanpeppers force-pushed the jonathanpeppers/ctrl-c-device-test branch from 996bb8f to 212f498 Compare May 7, 2026 21:09
@jonathanpeppers
Copy link
Copy Markdown
Member Author

Addressed in latest push:

pidof race condition (line 253): Good catch — StopAppAsync is fire-and-forget in OnCancelKeyPress, so am force-stop may not have completed by the time we check pidof. Changed to poll with retries (up to 10s) before asserting.

force-stop in finally (line 261): Added RunAdbCommand("shell am force-stop ...") in the finally block to clean up the app on the device if the test fails before Ctrl+C takes effect, preventing contamination of subsequent tests.

The other comments are not relevant for this PR:

  • Windows skip: These tests only run on macOS CI. SendCtrlC already throws PlatformNotSupportedException which is sufficient.
  • Exit code 130 on Windows: 130 is a reasonable "canceled" exit code on all platforms. No consumer parses this value.

Add a device integration test that verifies Ctrl+C (SIGINT) sent to
`dotnet run` properly stops the Android app on the device/emulator.

The test:
1. Builds and launches an Android app via `dotnet run` with WaitForExit
2. Waits for app logcat output confirming launch
3. Verifies the app PID exists on the device
4. Sends SIGINT via Process.SendCtrlC() extension method
5. Asserts the process exits gracefully
6. Verifies Microsoft.Android.Run's Ctrl+C handler ran ("Stopping application...")
7. Confirms the app is no longer running on the device

Also adds a `SendCtrlC()` extension method on `Process` in
`ProcessExtensions.cs`. On Unix/macOS, it uses `pgrep` to find all
descendant processes and sends SIGINT to the entire process tree,
simulating what a real terminal Ctrl+C does (SIGINT to the whole
foreground process group). Without this, child processes like
`Microsoft.Android.Run` (launched by `dotnet run` via MSBuild) would
not receive the signal.

Fixes a bug in `Microsoft.Android.Run` where `OperationCanceledException`
from `GetAppPidAsync()` would escape through the top-level catch block
when Ctrl+C was pressed, printing "Error: The operation was canceled."
instead of exiting cleanly.

Fixes: #11264

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers jonathanpeppers force-pushed the jonathanpeppers/ctrl-c-device-test branch from 212f498 to 66c28ba Compare May 7, 2026 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants